Infrastructure/11293 phpcs rule fullstop by ankitrox · Pull Request #13148 · google/site-kit-wp

ankitrox · 2026-07-20T15:18:09Z

Summary

Addresses issue:

Add a PHPCS rule to enforce full stops in PHPDoc comments #11293

Relevant technical choices

PR Author Checklist

My code is tested and passes existing unit tests.
My code has an appropriate set of unit tests which all pass.
My code is backward-compatible with WordPress 5.2 and PHP 7.4.
My code follows the WordPress coding standards.
My code has proper inline documentation.
I have added a QA Brief on the issue linked above.
I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

Run the code.
Ensure the acceptance criteria are satisfied.
Reassess the implementation with the IB.
Ensure no unrelated changes are included.
Ensure CI checks pass.
Check Storybook where applicable.
Ensure there is a QA Brief.
Ensure there are no unexpected significant changes to file sizes.

Merge Reviewer Checklist

Ensure the PR has the correct target branch.
Double-check that the PR is okay to be merged.
Ensure the corresponding issue has a ZenHub release assigned.
Add a changelog message to the issue.

github-actions · 2026-07-20T15:23:38Z

🤖 This comment is automatically updated by CI workflows. Each section is managed independently.

📦 Build files for 17b6b35:

Build files have been deleted.

🎭 Playwright reports for 17b6b35:

techanvil

Thanks @ankitrox, this is looking good. I've left some comments, please take a look.

techanvil · 2026-07-21T13:18:10Z

 	 * ## EXAMPLES
 	 *
-	 *     wp google-site-kit auth disconnect --id=11
+	 *     wp google-site-kit auth disconnect --id=11.


The rule should be disabled for this comment, and this change reverted.

techanvil · 2026-07-21T13:18:31Z

 	 *
 	 *     wp google-site-kit reset
-	 *     wp google-site-kit reset --persistent
+	 *     wp google-site-kit reset --persistent.


The rule should be disabled for this comment, and this change reverted.

techanvil · 2026-07-21T13:24:07Z

 		// which requires authorization, however script_loader_tag is also filtered
 		// to apply script_execution attributes for all users, so it must be excluded here.
-		// 'script_loader_tag',
+		// 'script_loader_tag',.


Let's remove this commented out line:

Suggested change

// 'script_loader_tag',.

techanvil · 2026-07-22T09:52:37Z

 		$config['headers']['User-Agent'] = Google_Proxy::get_application_name();

-		/** This filter is documented in wp-includes/class-http.php */
+		/** This filter is documented in wp-includes/class-http.php.*/


The rule should preserve trailing whitespace for single-line docblocks, i.e. this should be fixed as follows:

Suggested change

/** This filter is documented in wp-includes/class-http.php.*/

/** This filter is documented in wp-includes/class-http.php. */

techanvil · 2026-07-22T09:53:18Z

 		}

-		/** This filter is documented in wp-login.php */
+		/** This filter is documented in wp-login.php.*/


As above:

Suggested change

/** This filter is documented in wp-login.php.*/

/** This filter is documented in wp-login.php. */

techanvil · 2026-07-22T10:01:01Z

+ * @package Google\Site_Kit\Sniffs\Commenting
+ */
+
+namespace Google\Site_Kit\Sniffs\Commenting;


This should use the same namespace as RequireDocTagDescriptionSniff.

Suggested change

namespace Google\Site_Kit\Sniffs\Commenting;

namespace Google\Site_Kit\PHPCS\Sniffs\Commenting;

techanvil · 2026-07-22T10:07:08Z

+	 * @param string $content Token content.
+	 * @return string Updated token content.
+	 */
+	private function append_full_stop_preserve_newline( $content ) {


I think this method needs attention to preserve the whitespace I flagged in my comments above.

@ankitrox thanks for updating the logic, but let's also update the method name to be more clear:

Suggested change

private function append_full_stop_preserve_newline( $content ) {

private function append_full_stop_preserve_whitespace( $content ) {

ankitrox · 2026-07-22T14:08:28Z

Thank you @techanvil for reviewing the PR. I have updated the rule to preserve the single whitespace in single line comment.
Over to you for another pass.

techanvil

Thanks @ankitrox, please see my comments.

techanvil · 2026-07-22T14:22:57Z

 	 *
 	 * ## EXAMPLES
 	 *
+	 * phpcs:ignore PHPCS.Commenting.DocCommentFullStop.MissingFullStop -- Command examples should not include trailing punctuation.


Can you move this phpcs:ignore line above the start of the /** block? It interferes with the doc readability where it is.

techanvil · 2026-07-22T14:23:20Z

 	 * ## EXAMPLES
 	 *
 	 *     wp google-site-kit reset
+	 * phpcs:ignore PHPCS.Commenting.DocCommentFullStop.MissingFullStop -- Command examples should not include trailing punctuation.


As above, please move this above the /** start of the block.

ankitrox · 2026-07-22T14:54:51Z

Thanks @techanvil
Moved those phpcs rules on top as suggested.

techanvil

Hi @ankitrox, this needs a couple of last tweaks as per my comments.

Also - the QAB needs updating, as the sniff is now registered as PHPCS.Commenting.DocCommentFullStop instead of Site_Kit.Commenting.DocCommentFullStop.

techanvil · 2026-07-23T11:04:43Z

+	 * @param string $content Token content.
+	 * @return string Updated token content.
+	 */
+	private function append_full_stop_preserve_newline( $content ) {


@ankitrox thanks for updating the logic, but let's also update the method name to be more clear:

Suggested change

private function append_full_stop_preserve_newline( $content ) {

private function append_full_stop_preserve_whitespace( $content ) {

techanvil · 2026-07-23T11:05:02Z

+	}
+
+	/**
+	 * Appends a full stop while preserving any trailing newline characters.


As per my comment below:

Suggested change

* Appends a full stop while preserving any trailing newline characters.

* Appends a full stop while preserving any trailing whitespace.

techanvil · 2026-07-23T11:05:30Z

 	 * @param array $assoc_args Array of associated arguments.
 	 */
 	public function disconnect( $args, $assoc_args ) {
+		// phpcs:enable PHPCS.Commenting.DocCommentFullStop.MissingFullStop


Looks like this line got added by accident:

Suggested change

// phpcs:enable PHPCS.Commenting.DocCommentFullStop.MissingFullStop

Sorry, I made a mistake here - this line is needed to re-enable the rule. I misread it as a phpcs:disable line. I'll restore it myself to save a round trip.

ankitrox · 2026-07-23T11:42:04Z

Thanks @techanvil
Updated the QAB and addressed the PR comments.

…FullStop rule.

techanvil

LGTM, thanks @ankitrox!

✅

ankitrox added 2 commits July 20, 2026 20:37

Add phpcs rule.

25ec211

Fix lint issue in files.

e20471d

techanvil requested changes Jul 22, 2026

View reviewed changes

ankitrox added 2 commits July 22, 2026 19:24

Merge branch 'develop' into infrastructure/11293-phpcs-rule-fullstop.

c5f03fb

Preserve single space in single line comment.

3c358c1

techanvil requested changes Jul 22, 2026

View reviewed changes

Move phpcs disable rule.

b1fcdc7

techanvil requested changes Jul 23, 2026

View reviewed changes

ankitrox added 2 commits July 23, 2026 17:02

Merge branch 'develop' into infrastructure/11293-phpcs-rule-fullstop.

78ff2c5

Update method name.

bde8d01

Restore line to re-enable PHPCS.Commenting.DocCommentFullStop.Missing…

17b6b35

…FullStop rule.

techanvil approved these changes Jul 23, 2026

View reviewed changes

techanvil merged commit 81f9963 into develop Jul 23, 2026
34 checks passed

techanvil deleted the infrastructure/11293-phpcs-rule-fullstop branch July 23, 2026 13:23

	/** This filter is documented in wp-includes/class-http.php.*/
	/** This filter is documented in wp-includes/class-http.php. */

	/** This filter is documented in wp-login.php.*/
	/** This filter is documented in wp-login.php. */

	namespace Google\Site_Kit\Sniffs\Commenting;
	namespace Google\Site_Kit\PHPCS\Sniffs\Commenting;

	private function append_full_stop_preserve_newline( $content ) {
	private function append_full_stop_preserve_whitespace( $content ) {

	* Appends a full stop while preserving any trailing newline characters.
	* Appends a full stop while preserving any trailing whitespace.

Uh oh!

Conversation

ankitrox commented Jul 20, 2026 • edited by techanvil Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Relevant technical choices

PR Author Checklist

Code Reviewer Checklist

Merge Reviewer Checklist

Uh oh!

github-actions Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

techanvil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankitrox commented Jul 22, 2026

Uh oh!

techanvil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankitrox commented Jul 22, 2026

Uh oh!

techanvil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankitrox commented Jul 23, 2026

Uh oh!

techanvil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ankitrox commented Jul 20, 2026 •

edited by techanvil

Loading

github-actions Bot commented Jul 20, 2026 •

edited

Loading